Skip to content

Conversation

@kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Jan 21, 2026

This pipes the fdv1 fallback header, and the environment ID, through the data sources. It doesn't utilize either piece of information.

This is draft while I sort publishing the okhttp-evensource.


Note

Introduces header propagation and response/result refactors for FDv2 data sources.

  • Add HeaderConstants and plumb x-ld-fd-fallback and x-ld-envid through polling/streaming; set fdv1Fallback on FDv2SourceResult and pass environmentId to change-set translation
  • Refactor FDv2Requestor.FDv2PayloadResponse to include statusCode/isSuccess with success/failure/none factories; DefaultFDv2Requestor now returns these (304 -> none, non-2xx -> failure)
  • Update PollingBase and StreamingSynchronizerImpl to interpret new responses, treat HTTP errors without throwing, and emit results with fallback flag; extract env-id from headers/events
  • Extend FDv2SourceResult to carry fdv1Fallback and new factory usages across code
  • Bump dependency: okhttp-eventsource 4.1.0 -> 4.2.0
  • Expand/adjust tests for header propagation, HTTP error handling, ETag/304, and environment ID

Written by Cursor Bugbot for commit b20c621. This will update automatically on new commits. Configure here.

@kinyoklion kinyoklion marked this pull request as ready for review January 23, 2026 20:58
@kinyoklion kinyoklion requested a review from a team as a code owner January 23, 2026 20:58
if (response.code() == 304) {
logger.debug("FDv2 polling request returned 304: not modified");
future.complete(null);
future.complete(FDv2PayloadResponse.none(response.code()));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having null indicate no transfer I made an explicit none.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

case CHANGESET:
FDv2ProtocolHandler.FDv2ActionChangeset changeset = (FDv2ProtocolHandler.FDv2ActionChangeset) action;
try {
// TODO: Environment ID.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback header lost in serialization error paths

Medium Severity

In handleMessage, when SerializationException or protocol handler exceptions occur, interruptedWithException(e, ...) is called which uses getFallback(Exception e). This method only extracts the fallback header from StreamHttpErrorException, returning false for all other exception types. However, the MessageEvent event variable is available and contains the headers. Other error paths in the same method correctly use getFallback(event) (lines 257, 267, 281, 305), but these two catch blocks lose the header information.

Additional Locations (2)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants